feat(data-table): sticky columns and column settings#382
Conversation
|
Overall this looks good to me, but I have one strong concern about adding The main issue is that the relationship between They are also effectively mutually exclusive, but that is only enforced by a runtime warning and a precedence rule where More broadly, I think row interaction should be expressed through a single API surface rather than split across multiple top-level props. Otherwise users have to learn when to use For this PR, I would prefer to keep If we later want a first-class row navigation API with explicit modes, I think that should be a separate design discussion or follow-up PR. For example, I could imagine something like: type RowInteraction<TRow> =
| { type: "click"; onClick: (row: TRow) => void }
| { type: "link"; href: (row: TRow) => string; primaryColumnId?: string };
useDataTable({
columns,
data,
rowInteraction: { type: "link", href: (row) => `/orders/${row.id}` },
});That would make the modes explicit and avoid having both |
|
Thanks @IzumiSy — agree on all points. I've removed Row interaction is split into a separate design discussion: tailor-inc/platform-planning#1498, seeded with your Also noted from Slack: @sean confirmed localStorage is fine for persistence, so that's settled here. |
There was a problem hiding this comment.
Reviewed the current columns-only scope (noting rowHref was dropped in de74a41), via a local QA pass + adversarial review. Marking as Comment — not requesting changes.
One item I'd flag as a potential DevX blocker: pinning any content column flips the whole table to table-fixed, which silently requires every column to declare a width or it collapses/overlaps — details inline on data-table.tsx, and I'll attach a short video of the collapse. The rest are hardening: an off-by-one in drag-reorder, a width-less-pin UI/table mismatch, an unguarded onClickRow vs custom interactive cells, a positional column-key edge case, and notes on tableId persistence/uniqueness.
col-sticky-table.mp4
Credit where due — the pin offset math, the localStorage parse/validate/reconcile path, and the column-order self-healing are all solid.
| <Table.Cell | ||
| style={style} | ||
| className={className} | ||
| onClick={(e) => e.stopPropagation()} |
There was a problem hiding this comment.
Now that the row is clickable via onClickRow (the onClick on <Table.Row> just above), any click that bubbles up from a cell fires the row handler. The built-in selection cell (here) and the actions cell below guard against that with e.stopPropagation() — but a consumer who renders a <button> / toggle / inline menu inside a normal column via render gets no such guard, so clicking it also triggers onClickRow. Worth either wrapping cell content so interactive children stop propagation automatically, or documenting that custom interactive cells must call e.stopPropagation().
There was a problem hiding this comment.
onClickRow is pre-existing and left unchanged by this now columns-only PR. The row-interaction redesign — including how interactive render cells coexist with a clickable row (stopPropagation / guard, or an explicit API) — is tracked in platform-planning#1498. Leaving this open as a pointer into that work rather than resolving it here.
…ening Round-2 review follow-ups on the columns PR (#382): - Measure-based sticky offsets: drop the table-wide `table-fixed` switch and compute offsets from measured header-cell widths (PinMeasureContext + an isomorphic layout effect + ResizeObserver). Pinning no longer requires a `width`, so inferColumns/content-sized tables no longer collapse. - Fix off-by-one in same-zone downward drag-reorder in ColumnSettings. - Warn on duplicate `tableId` (a shared localStorage key clobbers layout); docs note on uniqueness. - Balance the row-actions column padding (was 24px right / 8px left). - ColumnSettings drop indicator is now a floating line between rows. Docs, catalogue skill, changeset, and the demo lab updated to match. The onClickRow-vs-custom-interactive-cells item is deferred to the row-interaction issue (tailor-inc/platform-planning#1498). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks for the thorough pass @interacsean — pushed fixes in ✅ table-fixed DevX blocker (the big one). Removed the table-wide ✅ Off-by-one drag reorder. Same-zone downward drops now decrement the insert index ( ✅ Width-less pin UI/table mismatch. Resolved by the measure-based change above — a width-less column now genuinely pins in both the popover and the table (no more phantom pin). ✅ Duplicate ↪️ onClickRow + custom interactive cells. Pre-existing ➖ Positional column-key edge case. Left as-is: it only affects columns with neither Also, while testing: balanced the row-actions column padding (kebab was pushed left) and reworked the ColumnSettings drop indicator into a floating line between rows. 🙏 |
… navigation Adds three DataTable capabilities driven by the "too many columns / hard to reach the View button" feedback (platform-planning#1489): - Pinned/sticky columns via `pin: "left" | "right"` (selection auto-pins left, row-actions auto-pins right) with a scroll-aware freeze shadow. - `DataTable.ColumnSettings` popover: show/hide, drag-reorder, and pin columns by dragging between Fixed left / Scrollable / Fixed right zones. - Per-user column layout (visibility, order, pinning) persisted to localStorage via a new `tableId` option. - `rowHref` for accessible whole-row navigation (primary cell as a real <Link>), replacing per-row "View" buttons; `onClickRow` kept for non-navigation. Also: `Table.Root` gains an optional `containerRef`; docs, catalogue skill, and a demo lab page updated; changeset added. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…e dragging The insertion line was a flow element, so inserting it shifted the rows under the cursor, which recomputed the drop target on every reflow — a flicker loop. Draw the indicator as an inset box-shadow on the row instead (no reflow), and remove the inter-row gap so the end-of-section handler no longer fires as the cursor passes between rows. Empty zones get a dashed placeholder. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Hovering a column's checkbox shows "Hide column" (when visible) or "Show column" (when hidden). Adds showColumn/hideColumn i18n keys (en + ja). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Row interaction is being split out of this PR per review (@IzumiSy): the `rowHref` + `onClickRow` split is a confusing public API, and row interaction should be one explicit surface designed separately. Removes `rowHref` / `primaryColumnId` and the navigation rendering (NavigableRows / <Link> wrap / useNavigate), keeping the existing `onClickRow` untouched. `rowHref` was never released, so this is not a breaking change. Column visibility, sticky/pinned columns, ColumnSettings, tableId persistence, and the Table.Root containerRef are unchanged. Docs, catalogue skill, demo lab page, tests, and changeset updated to columns-only. Row-interaction redesign tracked in tailor-inc/platform-planning#1498. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ening Round-2 review follow-ups on the columns PR (#382): - Measure-based sticky offsets: drop the table-wide `table-fixed` switch and compute offsets from measured header-cell widths (PinMeasureContext + an isomorphic layout effect + ResizeObserver). Pinning no longer requires a `width`, so inferColumns/content-sized tables no longer collapse. - Fix off-by-one in same-zone downward drag-reorder in ColumnSettings. - Warn on duplicate `tableId` (a shared localStorage key clobbers layout); docs note on uniqueness. - Balance the row-actions column padding (was 24px right / 8px left). - ColumnSettings drop indicator is now a floating line between rows. Docs, catalogue skill, changeset, and the demo lab updated to match. The onClickRow-vs-custom-interactive-cells item is deferred to the row-interaction issue (tailor-inc/platform-planning#1498). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
13ed93d to
80aac8a
Compare
There was a problem hiding this comment.
Pull request overview
Adds column-focused UX improvements to the core DataTable component: sticky/pinned columns, a user-facing column settings popover (show/hide, reorder, pin/unpin), and optional per-user persistence via localStorage keyed by a new tableId. Also expands the underlying Table.Root API to expose its scroll container ref, and updates docs/examples/tests accordingly.
Changes:
- Introduces sticky/pinned column rendering (including selection/actions auto-pinning and scroll-aware “freeze shadow”) and pin-related APIs (
Column.pin,pinnedColumns,setPin). - Adds persisted column layout support via
tableId(visibility/order/pinning stored inlocalStorage) and new ordering APIs (columnOrder,moveColumn,setColumnOrder). - Adds
DataTable.ColumnSettingsUI + supporting docs, examples (DataTable Lab), and test coverage.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/components/table.tsx | Adds containerRef to Table.Root so consumers can observe the scroll container. |
| packages/core/src/components/data-table/use-persistent-column-state.ts | New SSR-safe localStorage persistence hook for column layout state. |
| packages/core/src/components/data-table/use-data-table.ts | Replaces in-memory hidden-column state with persisted visibility/order/pinning and exposes new APIs. |
| packages/core/src/components/data-table/use-data-table.test.ts | Adds tests for column ordering/pinning and persistence behavior. |
| packages/core/src/components/data-table/types.ts | Adds Column.pin, UseDataTableOptions.tableId, and new return fields for order/pinning. |
| packages/core/src/components/data-table/i18n.ts | Adds labels for column settings UI. |
| packages/core/src/components/data-table/data-table.tsx | Implements sticky column layout, width measurement, scroll-edge shadowing, and exposes DataTable.ColumnSettings. |
| packages/core/src/components/data-table/data-table.test.tsx | Updates truncate assertions and adds pinned-column behavior tests. |
| packages/core/src/components/data-table/data-table-context.tsx | Extends context with order/pinning APIs. |
| packages/core/src/components/data-table/column-settings.tsx | New DataTable.ColumnSettings popover UI for show/hide, reorder, and pin/unpin. |
| packages/core/src/components/data-table/column-settings.test.tsx | Adds UI-level tests for column settings behavior (toggle, drag/drop pinning, show/hide all). |
| examples/vite-app/src/routes.generated.ts | Adds route typing for the new DataTable Lab page. |
| examples/vite-app/src/pages/data-table-lab/page.tsx | Adds a demo “DataTable Lab” page exercising the new column features. |
| examples/vite-app/src/App.tsx | Adds sidebar navigation entry for DataTable Lab. |
| docs/components/data-table.md | Documents pinning, column settings, and tableId persistence. |
| catalogue/src/pattern/list/dense-scan/PATTERN.md | Updates pattern guidance to recommend pinning + column settings for wide lists. |
| catalogue/src/fundamental/components.md | Updates DataTable catalogue documentation with pinning and column settings usage. |
| .changeset/data-table-columns-and-row-nav.md | Minor changeset describing the new DataTable column features and Table.Root.containerRef. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- usePersistentColumnState: reset to defaults when `tableId` is cleared instead of keeping the previous table's persisted layout (in-memory mode shouldn't inherit a prior id's state). - moveColumn: reconcile column order from `prev` inside the state updater (via a shared `reconcileColumnOrder` helper) rather than a captured `columnOrder`, so reorder calls batched before a re-render compose instead of clobbering. Adds regression tests for both. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Addressed the Copilot review comments in
Both are low-severity edge cases (the built-in drag UI uses |
Collapse the two footer buttons into a single right-aligned button whose label and action follow visibility state: "Hide all" when every column is visible, "Show all" otherwise. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…le index `computePinLayout` derived each column's render key from its position in the filtered/reordered *visible* array, while `useDataTable`/`ColumnSettings` store order/visibility/pin state keyed off the *definition-order* index. For a column with neither `id` nor `label` (key falls back to `String(index)`), the two index spaces diverge once a sibling is hidden or reordered, silently detaching the column's stored pin/visibility. Resolve keys once into a definition-order `col → key` map (`buildColumnKeys`) and thread it through `computePinLayout` (by column reference), so the key is stable regardless of visible position. Columns with an `id` or `label` were unaffected. Addresses interacsean's review on PR #382. Adds a regression test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Summary
Column controls for
DataTable, from the "too many columns / constant scroll-right" feedback in platform-planning#1489.pin: "left" | "right"on aColumn(widthoptional but recommended — sticky offsets are measured from the rendered layout). Selection auto-pins left, row-actions auto-pins right, with a scroll-aware freeze shadow that appears once content scrolls under the edge.DataTable.ColumnSettings— a "Columns" toolbar popover with three drop zones (Fixed left / Scrollable / Fixed right): show/hide via checkboxes, drag to reorder, and drag between zones to pin/unpin.tableIdto persist each user's visibility/order/pinning tolocalStorage. Per-user preference, not URL. Omit for in-memory only.Table.Rootgains an optionalcontainerRef.Docs (
docs/components/data-table.md), the catalogue skill (components.md/list-dense-scan→ regeneratedapp-shell-patterns), a demo DataTable Lab page, tests, and aminorchangeset are included.Persistence
Resolved per @sean on Slack — localStorage is fine for now (per-device layout is acceptable; a synced user-settings store would be more complexity for little benefit).
Try it
pnpm dev→ DataTable Lab in the sidebar: open Columns to show/hide, drag to reorder, drag between zones to pin; scroll horizontally to see pinned columns + freeze shadow.🤖 Generated with Claude Code